-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace instance variable access with call to .elf. #7996
Replace instance variable access with call to .elf. #7996
Conversation
…access with call to .elf. patcher.elf was introduced in gem release v1.1.0.
We don’t have version requirements for the other gems. Should we update Gemfile.lock instead? |
Perhaps both? We won't be able to function with versions of @rmNULL, could you do that? |
Yeah, but it should always update to the latest in the lockfile, since setting I note that patchelf isn't currently specified in |
For consistency with the rest of the |
Good points. No objections here! |
If anything, we should:
This is fine because (and correct me if I'm wrong) we always check for When we flip the switch so everyone uses |
Yeah, this sounds preferable to me. I can't recall why we didn't do this in the first place, other than maybe not wanting to install |
Now that |
Would it affect Dependabot at all? |
I think dependabot would push updates to the lockfile as usual. |
I asked since Dependabot can purge dependencies no longer used, and I wasn't sure how it did that internally. |
I think dependabot basically does |
👍 on this. I missed that point earlier - I thought this was about committing the lockfile only. |
The conditional |
Co-authored-by: Shaun Jackman <sjackman@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
brew style
with your changes locally?brew tests
with your changes locally?.elf
was introduced in gem release v1.1.0.Resolves #7678 (comment)